src: add event loop based metrics into node#62935
Conversation
edb66ea to
e7093c8
Compare
6f5cdca to
bc0a59c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #62935 +/- ##
==========================================
- Coverage 89.65% 89.64% -0.02%
==========================================
Files 708 708
Lines 220413 220546 +133
Branches 42275 42290 +15
==========================================
+ Hits 197607 197702 +95
- Misses 14658 14675 +17
- Partials 8148 8169 +21
🚀 New features to boost your workflow:
|
bc0a59c to
27ead1e
Compare
27ead1e to
3b21617
Compare
71f121f to
88fce8d
Compare
50f8eab to
29b708d
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
LGTM, while I would still like to get another LG who works on CPP frequently
Add test coverage for the per iteration event loop delay histogram start and stop callbacks Use disitinct debug tracking keys for the event loop delay histogram callbacks.
Use the super Close method instead of duplicating code
Added extra test to add coverage of the overall functionality of the new ELD histogram.
Update docs to match the event loop delay sampling changes.
The C++ class `ELDHistogram` is the per-iteration sampling implementation that sits next to `IntervalHistogram` (the timer-based one). Its name collided with the JS exposed `ELDHistogram` class that `monitorEventLoopDelay()` returns regardless of sampling mode, making the layering confusing: "ELDHistogram" was both the generic JS concept and the name of one specific C++ backing.
align perf_hooks docs after renaming internal ELDHistogram to IterationHistogram
Added shared templates for duplicated start/stop logic in IntevalHistogram and IterationHistogram.
7666e61 to
3f20e89
Compare
bengl
left a comment
There was a problem hiding this comment.
LGTM, modulo discussion of semver level. That's non-blocking though, since this can land as major if desired.
|
Per discussion offline w/ @BridgeAR, yeah it's a docs-only change, so removing the label. |
Commit Queue failed- Loading data for nodejs/node/pull/62935 ✔ Done loading data for nodejs/node/pull/62935 ----------------------------------- PR info ------------------------------------ Title src: add event loop based metrics into node (#62935) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch pabloerhard:pabloerhard/add-event-loop-metrics -> nodejs:main Labels c++, semver-minor, needs-ci Commits 9 - perf_hooks: sample delay per event loop iteration - perf_hooks: test event loop delay fast api callbacks - perf_hooks: format cpp - perf_hooks: use super Close method - test: improve event loop delay sample-per-iteration coverage - doc: update perf_hooks histogram docs - perf_hooks: rename internal ELDHistogram to IterationHistogram - doc: align perf_hooks histogram class name with implementation - perf_hooks: deduplicate histogram Start/Stop via shared templates Committers 1 - Pablo Erhard <pablo.erhardhernandez@datadoghq.com> PR-URL: https://github.com/nodejs/node/pull/62935 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/62935 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Fri, 24 Apr 2026 18:31:31 GMT ✔ Approvals: 3 ✔ - Bryan English (@bengl): https://github.com/nodejs/node/pull/62935#pullrequestreview-4565608564 ✔ - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/62935#pullrequestreview-4561486814 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/62935#pullrequestreview-4565515487 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-06-24T11:12:09Z: https://ci.nodejs.org/job/node-test-pull-request/74399/ - Querying data for job/node-test-pull-request/74399/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 62935 From https://github.com/nodejs/node * branch refs/pull/62935/merge -> FETCH_HEAD ✔ Fetched commits as 6f11fe7d67c1..3f20e89304dc -------------------------------------------------------------------------------- [main 1ae0f285c6] perf_hooks: sample delay per event loop iteration Author: pabloerhard <pabloerhard02@gmail.com> Date: Fri Apr 24 14:28:07 2026 -0400 7 files changed, 288 insertions(+), 13 deletions(-) [main 6e4c7e5150] perf_hooks: test event loop delay fast api callbacks Author: pabloerhard <pabloerhard02@gmail.com> Date: Thu Apr 30 13:54:56 2026 -0400 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-perf-hooks-monitor-event-loop-delay-fast-calls.js [main d46716593b] perf_hooks: format cpp Author: pabloerhard <pabloerhard02@gmail.com> Date: Thu Apr 30 19:33:22 2026 -0400 3 files changed, 30 insertions(+), 49 deletions(-) [main 9aa569e8d4] perf_hooks: use super Close method Author: pabloerhard <pabloerhard02@gmail.com> Date: Tue May 19 17:20:45 2026 -0400 2 files changed, 2 insertions(+), 17 deletions(-) [main b033f8fef1] test: improve event loop delay sample-per-iteration coverage Author: pabloerhard <pabloerhard02@gmail.com> Date: Wed May 27 15:38:42 2026 -0400 1 file changed, 83 insertions(+) [main 81acec265c] doc: update perf_hooks histogram docs Author: pabloerhard <pabloerhard02@gmail.com> Date: Wed May 27 15:48:50 2026 -0400 1 file changed, 20 insertions(+), 8 deletions(-) [main 27fd9191f7] perf_hooks: rename internal ELDHistogram to IterationHistogram Author: Pablo Erhard <pablo.erhardhernandez@datadoghq.com> Date: Wed Jun 17 12:54:37 2026 -0400 5 files changed, 47 insertions(+), 45 deletions(-) [main 32b0132990] doc: align perf_hooks histogram class name with implementation Author: Pablo Erhard <pablo.erhardhernandez@datadoghq.com> Date: Wed Jun 17 12:55:39 2026 -0400 1 file changed, 9 insertions(+), 13 deletions(-) [main 8f7515eb61] perf_hooks: deduplicate histogram Start/Stop via shared templates Author: Pablo Erhard <pablo.erhardhernandez@datadoghq.com> Date: Wed Jun 17 13:43:59 2026 -0400 4 files changed, 49 insertions(+), 31 deletions(-) ✔ Patches applied There are 9 commits in the PR. Attempting autorebase. (node:355) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/18) Executing: git node land --amend --yes --------------------------------- New Message ---------------------------------- perf_hooks: sample delay per event loop iterationhttps://github.com/nodejs/node/actions/runs/28128367127 |
|
Landed in c5635b8 |
Add a samplePerIteration option to monitorEventLoopDelay that records event loop delay from libuv event loop iterations instead of the timer interval sampler. The default remains interval-based; existing uses of monitorEventLoopDelay() keep behaving the same unless the samplePerIteration option is passed through. Signed-off-by: Pablo Erhard <pablo.erhardhernandez@datadoghq.com> PR-URL: nodejs#62935 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
Adds an opt in into monitorEventLoopDelay(), this option adds support for per iteration sampling using libuv hooks, while preserving the existing timer based approach.
When the new samplePerIteration option is set to true, monitorEventLoopDelay() samples directly from event loop iterations instead of using the interval timer. When samplePerIteration is omitted or false, the existing interval based implementation remains unchanged, including resolution handling and the older interval based trace counters.
Motivation
The interval based implementation has two limitations that the new option addresses:
This is an alternative implementation to the one in #62934
This PR directly addresses the following issue #56064